Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

json doc generator now generates _index.json as it creates files. #3696

Closed
wants to merge 44 commits into from

Conversation

iapain
Copy link

@iapain iapain commented Jul 12, 2012

generate.js now accepts --output=<file> which tools/doc/json.js uses to write the output. If output file is missing then it executes callback. This was required to get the directory for new _index.json. Ideally, I left it for you to decide what to call this _index.json. There is already an index.json file which should not be generated at all. Additionally it sets title as first heading node text if applicable.

TODO

  • Add exclude files to avoid generation of index.json
  • Dependency map in Makefile (if subchapters like structure is required).

json doc generator now accepts output file as well.
json doc generator now generates index file automatically.
@iapain iapain closed this Jul 12, 2012
@iapain iapain reopened this Jul 12, 2012
@iapain
Copy link
Author

iapain commented Jul 12, 2012

Damn I cannot attach it to the issue. It's for #3668

@mainerror
Copy link

What's up with this pull request? Is it going to get merged?

@iapain
Copy link
Author

iapain commented Jul 31, 2012

It still needs some API related decisions from @isaacs. He urged me to write this patch but probably too busy with other stuff.

@mainerror
Copy link

I reckoned, just wanted to bump this a little.

@isaacs
Copy link

isaacs commented Oct 3, 2012

The index should be written to index.json, not _index.json.

Also, it's a bit odd having url be a an object. Maybe it should be just html: 'foo.html', json: 'foo.json' or something?

@iapain
Copy link
Author

iapain commented Oct 3, 2012

@isaacs The reason I kept _index.json was index.markdown translates into index.json but I guess it's no longer required. In last commit I have renamed it to index.json. Moreover url.html, url.json is now just html, json as you suggested.

@Nodejs-Jenkins
Copy link

Can one of the admins verify this patch?

@mainerror
Copy link

Indeed, it would be awesome to get this in, I don't quite understand what's taking eight months to review ...

@bnoordhuis
Copy link
Member

Yeah, because we're all just twiddling our thumbs while raking in the big paychecks...

Look at the number of open issues and pull requests, now realize that there are only two or three people working on node.js full time.

If you want to do something constructive, start triaging bug reports and reviewing PRs.

@mainerror
Copy link

@bnoordhuis I didn't say that you guys weren't doing something but eight months for such a minor PR?

I'm not actively working on Node.js, I have other projects I'm actively contributing to.

@iapain
Copy link
Author

iapain commented Mar 14, 2013

I must agree with @mainerror being a contributor it's very discouraging. I tried to bring it up few times on IRC to @isaacs in vain.

@bnoordhuis I totally respect what you guys are doing it's pretty cool and cutting edge but eventually it's core team who needs to organize and expand if it's required.

@@ -146,9 +159,62 @@ function doJSON(input, filename, cb) {
finishSection(current, stack[stack.length - 1]);
}

return cb(null, root)
if (outfile) {
writeOututToFile(root, filename, outfile, indexfile, writeToIndexFile);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? Should this be writeOutputToFile?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that's a typo. Will rectify in next commit.

@isaacs
Copy link

isaacs commented Mar 14, 2013

In general, it is better to light candles than to curse darkness. @mainerror, if you have comments on this patch, then share them. We welcome reviews from all parties. Though it does take some core team member to actually land it, you can help things along by providing feedback.

@iapain I'm a bit confused about what happens if just one of the files changes. Ie, if you've generated the index.json, and then we change doc/api/url.markdown, for instance. Will it generate a new index.json containing all of the previous files as well? How does it know to do index.json last?

@iapain
Copy link
Author

iapain commented Mar 14, 2013

@isaacs I'm not tracking anything, it just regenerates index.json. Should it keep track for changes?

@iapain
Copy link
Author

iapain commented Mar 14, 2013

@isaacs changes checked-in. I think now I got it what you were trying to say in previous comment. Actually writeToIndexFile checks if there is already an index.json file. It also check (in a way) that if it's proper index.json and then it update or create proper chapter entry into main json object which is written back to index.json

iapain and others added 6 commits March 14, 2013 21:48
The current implementation uses the arguments object in the Server()
constructor. Since both arguments to Server() are optional, there was a
high likelihood of accessing a non-existent element in arguments, which
carries a performance overhead. This commit replaces the arguments
object with named arguments.

Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Reviewed-by: Chris Dickinson <christopher.s.dickinson@gmail.com>
@@ -146,9 +159,64 @@ function doJSON(input, filename, cb) {
finishSection(current, stack[stack.length - 1]);
}

return cb(null, root)
if (outfile) {
writeOutputToFile(root, filename, outfile, indexfile, writeToIndexFile);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see the source of my confusion. writeToIndexFile performs async actions, but never calls cb. This probably needs to be something like the following:

writeOutputToFile(root, filename, outfile, indexfile, function(err, root, sourcefile, outfile) {
  if (err) return cb(err);
  writeToIndexFile(root, sourcefile, outfile, cb);
})

Julien Gilli and others added 11 commits March 16, 2015 15:55
'.' and '..' are directory specs and resolving urls with or without the
hostname with '.' and '..' should add a trailing slash to the end of the
url.

Fixes nodejs#8992.

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#9427
Conflicts:
	src/node_version.h
Conflicts:
	src/node_version.h
Port of io.js commit: nodejs/node@77f3586

Original commit message:

This updates the default cipher suite to an more secure list, which
prefers strong ciphers with Forward Secrecy. Additionally, it enables
`honorCipherOrder` by default.

Noteable effect of this change is that the insecure RC4 ciphers are
disabled and that Chrome negotiates a more secure ECDHE cipher.

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#14383
Add command line switches and environment variables to override
the default cipher suite in tls.js

`--cipher-list` and `NODE_CIPHER_LIST` can be used to completely
override the default cipher list with a given value.

`--enable-legacy-cipher-list` and `NODE_LEGACY_CIPHER_LIST` can
be used to reset the default cipher list back to a known legacy
value shipped in prior Node.js releases

A new `getLegacyCiphers` method on the tis module allows
programmatic access to the old cipher list defaults.

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#14383
Based on commit feedback, make the PrintHelp for
--enable-legacy-cipher-list less verbose.

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#14383
Per the commit feedback, fix up style nits and pass in the
isolate with the NODE_DEFINE_STRING_CONSTANT macro.

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#14383
typo and unnecessary options init

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#14383
Conflicts:
	src/node_version.h
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25274
@jasnell
Copy link
Member

jasnell commented Jun 1, 2015

@iapain @chrisdickinson ... I know it's been a while, but is this still something you want to pursue? If yes, the PR would need to be updated significantly. If not, we should likely go ahead and close.

jasnell and others added 2 commits June 3, 2015 22:34
Fixes: nodejs#5317

Improve the example in the documentation to show
that response content can be chunked across multiple
`data` events.

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25352
Fix misspelling of 'response' on line 453

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25472
@iapain
Copy link
Author

iapain commented Jun 4, 2015

@jasnell I will implement @chrisdickinson suggestions in neat way. Hold on.

@jasnell
Copy link
Member

jasnell commented Jun 4, 2015

@iapain 👍

socketpair and others added 10 commits June 4, 2015 09:31
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25341
Conflicts:
	doc/api/tls.markdown
	src/node_version.h
You must consume the data from the response object. nodejs#8443

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25471
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25453
The third sentence of the fifth paragraph of the documentation for
transform._transform() has several words omitted and makes no
sense. This fix fills in the missing words to clarify the passage.

Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#25371
json doc generator now accepts output file as well.
json doc generator now generates index file automatically.
@iapain
Copy link
Author

iapain commented Jun 18, 2015

@jasnell Ready to review.

@iapain
Copy link
Author

iapain commented Jun 18, 2015

@jasnell I've to close this PR and will send you another one shortly. Looks like git rebase messed up with previous commits.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet